Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added demo for BLE #2801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added demo for BLE #2801

wants to merge 1 commit into from

Conversation

robertc2000
Copy link

Summary

Created a minimal demo application for Bluetooth. It works by displaying some data over Bluetooth and the user can connect to the device running it on NuttX and read this data.

This can work in 2 scenarios: a real scenario, in which data from the BME680 (temperature, humidity, etc..) is used and the user will be able to read it. The second scenario involves displaying some dummy data (hardcoded values) for testing purposes.

This application can serve as an example for users implementing more complex application using Bluetooth on NuttX.

Testing

I used the ESP32-Sparrow (which includes the BME680 sensor) and the ESP32-devkitc boards for testing.

The Bluetooth stack starts by default with advertising when it initializes, however I noticed that enabling advertising in the stack does not work (at least on ESP32). However, when scanning is also enabled, the device starts advertising. Therefore, in our application, during the BLE services initialization part, I also enable scanning as a temporary workaround in order to make sure the device advertises and the user can see it and connect to it.

@nuttxpr
Copy link

nuttxpr commented Oct 29, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

NuttX PR Requirements Review

While the provided information is a good start, it does not fully meet the NuttX PR requirements. Here's a breakdown:

Strengths:

  • Clear summary: The summary clearly explains the purpose of the change (new demo application), its functionality (Bluetooth data display), and potential use cases (real and dummy data scenarios).
  • Testing information: You mentioned the target boards used for testing (ESP32-Sparrow, ESP32-devkitc) and provided context on a workaround implemented for advertising issues.

Areas for improvement:

  • Missing details in Summary:
    • Functional part: Specify which part of the codebase is affected (e.g., apps/examples/bluetooth).
    • How it works (technical details): Briefly explain the implementation details, like which Bluetooth profiles are used, how data is transmitted/received.
    • Issue/PR references: Link any related issues or PRs (if applicable).
  • Incomplete Impact assessment:
    • New feature: Clearly state that this is a new demo application.
    • User Impact: Describe how users can utilize this new application (e.g., building and flashing instructions, commands to run).
    • Build impact: Mention any new dependencies or build configuration changes required.
    • Documentation impact: Confirm if documentation is updated/provided to reflect this new application.
    • Other impacts: Address all other impact points (hardware, security, compatibility) even if they are not applicable ("N/A" is acceptable).
  • Missing Testing logs: Provide actual testing logs (before and after the change) to demonstrate the application's functionality.

Recommendations:

  1. Expand the Summary section with the missing technical details and references.
  2. Thoroughly address all points in the Impact section, even if the impact is "NO" or "N/A".
  3. Include actual testing logs demonstrating the application's behavior before and after your changes.

By providing this additional information, your PR will be more comprehensive and better aligned with NuttX requirements.

@cederom
Copy link

cederom commented Oct 29, 2024

Very cool @robertc2000 !! :-) Could you please add some documentation too (https://github.com/apache/nuttx/tree/master/Documentation)? :-)

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please copy the PR message as commit message

@xiaoxiang781216
Copy link
Contributor

please fix the style issue report here:
https://github.com/apache/nuttx-apps/actions/runs/11580827866/job/32250505341?pr=2801

../nuttx/tools/checkpatch.sh -m -g d9e178aad022030224d1c95628cab1784a13a339..HEAD
/home/runner/work/nuttx-apps/nuttx-apps/apps/examples/ble/ble.c: error: No block comments found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/ble/ble.c:1:1: error: Missing file header comment block
Warning: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/ble/ble.c:1:1: warning: #include outside of 'Included Files' section
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/ble/ble.c:2:0: error: Path relative to repository other than "nuttx" must begin with the root directory
Warning: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/ble/ble.c:2:1: warning: #include outside of 'Included Files' section

you can run this command locally before updating the patch:

../nuttx/tools/checkpatch.sh -g HEAD~...HEAD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants